Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hashing context example #91920

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

dmipeck
Copy link
Contributor

@dmipeck dmipeck commented May 13, 2024

The example function produces invalid hashes for files that aren't a multiple of CHUNK_SIZE bytes in size

Fixes two issues:

  • replaces use of eof_reached() who's docs say to use file.get_position() < file.get_len() instead when looping
  • uses remaining number of bytes instead of CHUNK_SIZE if CHUNK_SIZE would be too large

@dmipeck dmipeck requested a review from a team as a code owner May 13, 2024 20:00
@Calinou Calinou added bug documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels May 13, 2024
@Calinou Calinou added this to the 4.3 milestone May 13, 2024
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing to the documentation!

CI is complaining about indentation. Make sure the code is indented with 4 spaces, unlike the rest of the XML file which uses tabs.

Also, the commit is linked to a different GitHub account than the one you used to submit the PR. This is fine, but I wanted to make sure it wasn't by mistake. For more information see: Why are my commits linked to the wrong user?

See which "email" you have configured currently: https://github.com/godotengine/godot/pull/91920.patch

doc/classes/HashingContext.xml Outdated Show resolved Hide resolved
doc/classes/HashingContext.xml Outdated Show resolved Hide resolved
doc/classes/HashingContext.xml Outdated Show resolved Hide resolved
doc/classes/HashingContext.xml Outdated Show resolved Hide resolved
doc/classes/HashingContext.xml Outdated Show resolved Hide resolved
@dmipeck dmipeck force-pushed the fix-hashing-context-example branch 4 times, most recently from e84a6f7 to 570455b Compare May 14, 2024 07:21
Example now works for any file size instead of just multiples of CHUNK_SIZE
Example also uses correct method for looping over file data
@dmipeck dmipeck force-pushed the fix-hashing-context-example branch from 570455b to d6715b4 Compare May 14, 2024 07:24
@Mickeon Mickeon requested a review from raulsntos June 26, 2024 20:38
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I missed the notification. Looks good, thanks!

@akien-mga akien-mga merged commit 51a6bc1 into godotengine:master Jun 27, 2024
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants